plugin_proxy: enable event_type specification for proxy plugins #11011
Conversation
This commit adds event_type to the flb_plugin_proxy_def struct. This allows for the setting of event_type to a value other than the default log only initialization. In the flb_plugin_proxy_register function default behaviour is enforced by checking for unset event_type vlaues. When unset (a value of 0) then log behaviour is defaulted. No need for incorrect value checking since this was not a set field in the past. Unset fields are set as current behavior dicates and future usecases using incorrect values can be treated as user error. Input use case does not use event-type and is unaffected by this change. Signed-off-by: jmccormick7 <[email protected]>
jmccormick7
requested review from
edsiper,
fujimotos and
koleini
as code owners
WalkthroughAdded a public Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Plugin provider
participant Proxy as Plugin proxy
participant Core as Fluent Bit core
participant Out as Output instance
App->>Proxy: provide flb_plugin_proxy_def (event_type maybe 0)
Proxy->>Core: flb_proxy_register_output(def)
rect rgba(220,235,255,0.6)
Core->>Out: create output instance
alt def.event_type == 0
Core->>Out: set Out.event_type = FLB_OUTPUT_LOGS
else
Core->>Out: set Out.event_type = def.event_type
end
end
Core-->>Proxy: registration complete
sequenceDiagram
autonumber
participant App as Plugin provider
participant Proxy as Plugin proxy
participant Core as Fluent Bit core
participant In as Input instance
App->>Proxy: provide flb_plugin_proxy_def
Proxy->>Core: flb_proxy_register_input(def)
rect rgba(220,255,220,0.6)
Core->>Core: create input instance
Core->>Core: mk_list_add(In) into config.in_plugins
end
Core-->>Proxy: registration complete
sequenceDiagram
autonumber
participant Core as Fluent Bit core
participant GoOut as test_logs_go plugin
Core->>GoOut: FLBPluginFlushCtx(ctx, data, length, tag)
rect rgba(255,245,220,0.6)
GoOut->>GoOut: decode records with decoder.GetRecord()
GoOut->>GoOut: format each record to string
GoOut->>Stdout: print lines
GoOut-->>Core: return FLB_OK
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas to pay extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… malloc Changing intialization to use calloc instead of malloc so that emtpy fields are 0 initialized. Specifically for the event_type addition. Signed-off-by: jmccormick7 <[email protected]>
|
If we proceed to address this issue, we need to send another PR into here https://github.com/chronosphereio/calyptia-plugin to handle event_type as well. |
@cosmo0920 I can open a PR in that repository tonight! |
…e-metric-proxy-plugins
|
@cosmo0920 How would you prefer I do testing beyond the unit tests? I've done some manual testing that I can document, but is there any formalized regression testing or tests for proxy plugins? |
|
How about adding runtime shell test cases with precompiled Golang shared objects here? https://github.com/fluent/fluent-bit/tree/master/tests/runtime_shell |
This commit adds a runtime shell test for the go plugin interface. A dummy go output plugin is added to the directories, and a test that builds the plugin shared object and then tests that it works and writes the output. This test ensures that the go proxy interface is working as expected. This was tested in the devcontainer using ctest. Signed-off-by: jmccormick7 <[email protected]>
|
@cosmo0920 I have added a runtime test that build and test a go plugin shared object. Please let me know if there are any improvements or other additions I can provide to speed up the timeline! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
tests/runtime_shell/go_plugins/build_test_plugins.sh (1)
98-98: Consider using FLB_BUILD variable instead of hardcoded path.The CMakeLists.txt now exports FLB_BUILD to the environment. Consider using
$FLB_BUILDinstead of constructing$BUILD_DIRto ensure consistency with the test environment.Apply this diff:
- CGO_ENABLED=1 GO111MODULE=on go build -buildmode=c-shared -v -ldflags="-s -w" -o $BUILD_DIR/test_logs_go.so logs_output.go + CGO_ENABLED=1 GO111MODULE=on go build -buildmode=c-shared -v -ldflags="-s -w" -o ${FLB_BUILD:-$BUILD_DIR}/test_logs_go.so logs_output.gotests/runtime_shell/proxy_logs_expect.sh (2)
19-19: Use FLB_BUILD variable for consistent path construction.The script hardcodes
$FLB_ROOT/build/test_logs_go.so, but the CMakeLists.txt now providesFLB_BUILDin the environment for this purpose.Apply this diff:
- $FLB_BIN -e $FLB_ROOT/build/test_logs_go.so -c $FLB_RUNTIME_SHELL_CONF/proxy_logs_test.conf > "$STDOUT_OUTPUT_FILE" 2>&1 & + $FLB_BIN -e $FLB_BUILD/test_logs_go.so -c $FLB_RUNTIME_SHELL_CONF/proxy_logs_test.conf > "$STDOUT_OUTPUT_FILE" 2>&1 &
22-22: Replace hardcoded sleep with signal file polling.The hardcoded
sleep 3is fragile and can cause flakiness in tests. Consider polling for the SIGNAL_FILE_PATH or checking the process output instead of using a fixed delay.tests/runtime_shell/go_plugins/test_logs_go.h (1)
1-92: Consider excluding auto-generated CGO headers from version control.This file is marked as auto-generated by cmd/cgo and should not be manually edited. Consider adding
*.hor specificallytest_logs_go.hto .gitignore in this directory, and generate it during the build process instead.tests/runtime_shell/go_plugins/logs_output.go (1)
28-28: Consider using the timestamp from GetRecord.The timestamp returned by
GetRecord(second return value) is currently ignored. While this is acceptable for a test plugin, consider whether timestamp information would be useful for test verification.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/runtime_shell/go_plugins/go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
tests/runtime_shell/CMakeLists.txt(2 hunks)tests/runtime_shell/conf/proxy_logs_test.conf(1 hunks)tests/runtime_shell/go_plugins/build_test_plugins.sh(1 hunks)tests/runtime_shell/go_plugins/go.mod(1 hunks)tests/runtime_shell/go_plugins/logs_output.go(1 hunks)tests/runtime_shell/go_plugins/test_logs_go.h(1 hunks)tests/runtime_shell/proxy_logs_expect.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
tests/runtime_shell/CMakeLists.txt
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit, the correct CMake flag for using system librdkafka is `FLB_PREFER_SYSTEM_LIB_KAFKA=ON`.
Applied to files:
tests/runtime_shell/CMakeLists.txt
📚 Learning: 2025-09-04T07:28:37.083Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 10832
File: src/wasm/CMakeLists.txt:112-131
Timestamp: 2025-09-04T07:28:37.083Z
Learning: In fluent-bit CMake files, the user cosmo0920 prefers treating Git as a command rather than a package, emphasizing that Git is not a pkg-config retrievable package but just a command.
Applied to files:
tests/runtime_shell/CMakeLists.txt
🧬 Code graph analysis (1)
tests/runtime_shell/go_plugins/test_logs_go.h (1)
tests/runtime_shell/go_plugins/logs_output.go (4)
FLBPluginRegister(12-15)FLBPluginInit(18-20)FLBPluginFlushCtx(23-39)FLBPluginExit(42-44)
🔇 Additional comments (7)
tests/runtime_shell/CMakeLists.txt (2)
18-18: LGTM!The addition of the proxy logs test to the test suite is appropriate.
31-32: LGTM!Adding the FLB_BUILD environment variable enables tests to reference the build directory, which is useful for locating built artifacts like the test plugin shared object.
tests/runtime_shell/conf/proxy_logs_test.conf (1)
1-21: LGTM!The test configuration is well-structured for validating the proxy logs functionality. The use of environment variables for the signal file path enables flexible test execution.
tests/runtime_shell/go_plugins/logs_output.go (2)
11-15: LGTM!The registration function correctly delegates to the fluent-bit-go output package with appropriate plugin name and description.
22-39: LGTM!The flush implementation correctly decodes records and outputs them to stdout for test verification. The error handling is appropriate for a test plugin.
tests/runtime_shell/go_plugins/go.mod (2)
3-3: The Go version 1.25.1 is valid and exists.The latest stable Go release is Go 1.25.3 (released October 13, 2025), and Go 1.25.1 was released prior to that. The code's specification of
go 1.25.1is legitimate, though not the latest version available. No changes are required.Likely an incorrect or invalid review comment.
6-6: Review comment is incorrect — PR 80 is open, not merged.PR 80 ("Enable metric output plugins") in fluent-bit-go is currently open as of October 10, 2025 and has not been merged. Additionally, the plugin code (
logs_output.go) does not use or requireevent_typesupport. The current dependency version is appropriate for the code's functionality.Likely an incorrect or invalid review comment.
Signed-off-by: jmccormick7 <[email protected]>
2 participants
This commit adds event_type to the flb_plugin_proxy_def struct. This allows for the setting of event_type to a value other than the default log only initialization.
In the flb_plugin_proxy_register function default behavior is enforced by checking for unset event_type values. When unset (a value of 0) then log behavior is defaulted. No need for incorrect value checking since this was not a set field in the past. Unset fields are set as current behavior dictates and future use cases using incorrect values can be treated as user error.
Input use case does not use event-type and is unaffected by this change.
Addresses #10995
This corresponding PR in the fluent-bit-go library enables the ability to allow Go plugins to register their event types PR 80
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
Bug Fixes
Tests
Chores